[libc++] Refactor bitset::{any, all} #128445
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
winner245
force-pushed
the
simplify-bitset-any-all
branch
from
30e237c to
097854d
Compare
winner245
force-pushed
the
simplify-bitset-any-all
branch
from
097854d to
509e87c
Compare
llvmbot
added
the
libc++
|
@llvm/pr-subscribers-libcxx Author: Peng Liu (winner245) ChangesThe Full diff: https://github.com/llvm/llvm-project/pull/128445.diff 1 Files Affected:
diff --git a/libcxx/include/bitset b/libcxx/include/bitset
index f905b6f274e3f..8d0a5ec67a748 100644
--- a/libcxx/include/bitset
+++ b/libcxx/include/bitset
@@ -224,8 +224,12 @@ protected:
return to_ullong(integral_constant < bool, _Size< sizeof(unsigned long long) * CHAR_BIT>());
}
- _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 bool all() const _NOEXCEPT;
- _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 bool any() const _NOEXCEPT;
+ _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 bool all() const _NOEXCEPT {
+ return std::find(__make_iter(0), __make_iter(_Size), false) == __make_iter(_Size);
+ }
+ _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 bool any() const _NOEXCEPT {
+ return std::find(__make_iter(0), __make_iter(_Size), true) != __make_iter(_Size);
+ }
_LIBCPP_HIDE_FROM_ABI size_t __hash_code() const _NOEXCEPT;
private:
@@ -388,40 +392,6 @@ __bitset<_N_words, _Size>::to_ullong(true_type, true_type) const {
return __r;
}
-template <size_t _N_words, size_t _Size>
-_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 bool __bitset<_N_words, _Size>::all() const _NOEXCEPT {
- // do middle whole words
- size_t __n = _Size;
- __const_storage_pointer __p = __first_;
- for (; __n >= __bits_per_word; ++__p, __n -= __bits_per_word)
- if (~*__p)
- return false;
- // do last partial word
- if (__n > 0) {
- __storage_type __m = ~__storage_type(0) >> (__bits_per_word - __n);
- if (~*__p & __m)
- return false;
- }
- return true;
-}
-
-template <size_t _N_words, size_t _Size>
-_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 bool __bitset<_N_words, _Size>::any() const _NOEXCEPT {
- // do middle whole words
- size_t __n = _Size;
- __const_storage_pointer __p = __first_;
- for (; __n >= __bits_per_word; ++__p, __n -= __bits_per_word)
- if (*__p)
- return true;
- // do last partial word
- if (__n > 0) {
- __storage_type __m = ~__storage_type(0) >> (__bits_per_word - __n);
- if (*__p & __m)
- return true;
- }
- return false;
-}
-
template <size_t _N_words, size_t _Size>
inline size_t __bitset<_N_words, _Size>::__hash_code() const _NOEXCEPT {
size_t __h = 0;
@@ -713,8 +683,8 @@ public:
_LIBCPP_HIDE_FROM_ABI bool operator!=(const bitset& __rhs) const _NOEXCEPT;
# endif
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 bool test(size_t __pos) const;
- _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 bool all() const _NOEXCEPT;
- _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 bool any() const _NOEXCEPT;
+ _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 bool all() const _NOEXCEPT { return __base::all(); }
+ _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 bool any() const _NOEXCEPT { return __base::any(); }
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 bool none() const _NOEXCEPT { return !any(); }
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 bitset operator<<(size_t __pos) const _NOEXCEPT;
_LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 bitset operator>>(size_t __pos) const _NOEXCEPT;
@@ -901,16 +871,6 @@ _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 bool bitset<_Size>::test(siz
return (*this)[__pos];
}
-template <size_t _Size>
-inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 bool bitset<_Size>::all() const _NOEXCEPT {
- return __base::all();
-}
-
-template <size_t _Size>
-inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 bool bitset<_Size>::any() const _NOEXCEPT {
- return __base::any();
-}
-
template <size_t _Size>
inline _LIBCPP_HIDE_FROM_ABI _LIBCPP_CONSTEXPR_SINCE_CXX23 bitset<_Size>
bitset<_Size>::operator<<(size_t __pos) const _NOEXCEPT {
|
ldionne
approved these changes
May 12, 2025
|
In terms of codegen (https://godbolt.org/z/cdEbbTTnY at -O2), I found that the previous implementation generates more efficient assembly as it directly checks the underlying storage. In contrast, my proposed implementation introduces function calls to |
ldionne
requested changes
May 14, 2025
winner245
force-pushed
the
simplify-bitset-any-all
branch
2 times, most recently
from
669261f to
28fb22d
Compare
winner245
changed the title
|
The new refactoring yields exactly the same code in terms of codegen, as verified on https://godbolt.org/z/xx8hb4sPM. |
ldionne
approved these changes
May 21, 2025
| _LIBCPP_HIDE_FROM_ABI size_t __hash_code() const _NOEXCEPT; | ||
|
|
||
| private: | ||
| struct __bit_not { |
There was a problem hiding this comment.
I wouldn't block this patch on it, however it might be nice to consider a follow-up refactoring.
We have a __bit_not in valarray here: https://github.com/llvm/llvm-project/blob/main/libcxx/include/valarray#L491
We could provide a single __bit_not in https://github.com/llvm/llvm-project/blob/main/libcxx/include/__functional/operations.h#L220 and use it from both valarray and bitset.
winner245
force-pushed
the
simplify-bitset-any-all
branch
from
28fb22d to
018d34e
Compare
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
winner245
force-pushed
the
simplify-bitset-any-all
branch
from
018d34e to
255b057
Compare
winner245
force-pushed
the
simplify-bitset-any-all
branch
from
255b057 to
f26d014
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This patch refactors the
all()andany()methods inbitsetto eliminate redundant code while preserving the performance. Code generation remains unchanged, as verified on Compiler Explorer.